-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add MySQL query metricset with lightweight module and SQL helper #18955
Conversation
💔 Tests FailedExpand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
e3661af
to
45e85bd
Compare
b14cf5f
to
c34a9fd
Compare
I'm thinking to remove the Having it as a lightweight module, it is easy to include it later. |
Pinging @elastic/integrations-services (Team:Services) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice feature! I left some comments but in general looks good!
876ab17
to
41941aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good, I think that is a good idea to have generic metricsets on the specific modules to have documentation together and a more consistent experience for users using an only db.
Changelog entries will be needed, there are many things here we should mention:
- New query metricset in the mysql module.
- New performance metricset in the mysql module.
- Add support for booleans in the generic sql module.
- Null values are not reported in the generic sql module.
Maybe for the developer changelog:
- New sql helper for metricbeat modules.
Very good comments indeed. Thanks! Will take a look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please try to regenerate the data.json
files after adding the host parser and the replace_underscores
option to the manifest.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
metricbeat/docs/modules/sql.asciidoc
Outdated
driver: "postgres" | ||
sql_query: "SELECT * FROM pg_catalog.pg_tables pt WHERE schemaname ='pg_catalog'" | ||
sql_response_format: table | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should fix the doc build
--- | |
---- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh but this is generated. I don't know where this is is coming from. @dedemorton probably will
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh please, I cannot believe it 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, pushed and fixed. Thank you very much 😃
…stic#18955) (cherry picked from commit d0f7058)
* upstream/master: (25 commits) [Elastic Agent] Send checkin payload to Fleet (elastic#19857) [Ingest Manager] Fixed tests across agent elastic#19877 [Ingest Manager] Fix serialization test elastic#19876 Fix service start type mapping in windows/service metricset (elastic#19551) ci: Change comment trigger detection method (elastic#19827) Add 21 autogenerated filesets from rsa2elk devices (elastic#19713) [Ingest Manager] Agent config cleanup (elastic#19848) libbeat/publisher/pipeline: fix data races (elastic#19821) Update monitoring-internal-collection.asciidoc (elastic#19422) (elastic#19697) [Elastic Agent] Trust exchange endpoint must bind to 127.0.0.1 (elastic#19861) Specify an ECS version in Auditbeat/Packetbeat/Winlogbeat (elastic#19159) Add azure billing metricset (elastic#19207) Add support for appinsights in the metricbeat azure module (elastic#18940) Add MySQL query metricset with lightweight module and SQL helper (elastic#18955) [Ingest Manager] Refuse invalid stream values in configuration (elastic#19587) Do not use vendor during integration tests (elastic#19839) LIBBEAT: Enhancement Convert dissected values from String to other basic data types and IP (elastic#18683) [Elastic Agent] Remove support for "logs" and only support logfile (elastic#19761) [CI] support windows-2012 (elastic#19773) Do not update go.mod during packaging and testing (elastic#19823) ...
What does this PR do?
This PR adds a SQL helper (at Metricbeat level) with logic to execute SQL queries and return their results. The module can use this abstraction layer to simply pass in the SQL driver and the queries they need to execute to later create Beats events.
This PR introduce:
sql
module.sql
module.query
Metricset in MySQL module which uses the SQL helper. This Metricset is to use it as a Lightweight "parent" module.performance
metricset using thequery
metricset as Lightweight parent module. Use it as a guidance about how to make it work.Removedoverview
metricset with a bunch of key metrics, also using Lightweight modules. This is the metricset to remove of this PR.sql
module was moved to thesql
helper.Why is it important?
The way the
sql
was working was a bit limited compared to the underlying power it has. Moving it one layer up from the modules, allows the use of its logic by any module while maintaining each module functionality regarding specific DSN or security, for example, instead of introducing all that logic into thesql
module.The way it works now also allows to create metricsets on CockroachDB module using lightweight modules.
Checklist
[ ] I have made corresponding change to the default configuration filesCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.How to test this PR locally
Related issues
Fixes: #18898
Implements: #15048